I2C IT mode #135
I2C IT mode #135
Conversation
This reverts commit 625fe2d.
ghost
self-assigned this
ghost
requested review from
ghost
mentioned this pull request
ghost
mentioned this pull request
cores/arduino/stm32/twi.c
Outdated
| @@ -101,7 +101,7 @@ | |||
| */ | |||
|
|
|||
| /* Family specific description for I2C */ | |||
| #define I2C_NUM (5) | |||
| #define I2C_NUM (4) | |||
There was a problem hiding this comment.
can this value be defined in the family / target specific files ?
There was a problem hiding this comment.
It could be something like this:
#if defined(STM32F7xx) || defined(STM32L4xx)
#define I2C_NUM (4)
#elif defined(STM32F0xx) || defined(STM32F1xx) || defined(STM32L1xx)
#define I2C_NUM (2)
#else // STM32F2xx || STM32F3xx || STM32F4xx || STM32L0xx
#define I2C_NUM (3)
#endif
| i2c_t *obj = get_i2c_obj(hi2c); | ||
|
|
||
| if(obj->isMaster == 0) { | ||
| HAL_I2C_EnableListen_IT(hi2c); |
There was a problem hiding this comment.
can you comment or explain this ErrorCallback management ?
If there is an error, shouldn't we report the error to application ?
There was a problem hiding this comment.
In master mode, the callback is useless because the error is reported to the Arduino API with i2c_master_write() and i2c_master_read().
In slave mode, there is no mechanism in Arduino API to report an error so the error callback forces the slave to listen again.
There was a problem hiding this comment.
Ok Good. So please explain this either with comments in the code, or in the commit message, this is useful information that we cannot guess when reading the code
There was a problem hiding this comment.
Above in the header of the function.
|
|
||
| obj->irq = I2C1_EV_IRQn; | ||
| #if !defined(STM32F0xx) && !defined(STM32L0xx) | ||
| obj->irqER = I2C1_ER_IRQn; |
There was a problem hiding this comment.
one new proposal : in every place where I2Cx_ER_IRQn is being used couldn't you use
#if defined (I2C1_ER_IRQn)
obj->irqER = I2C1_ER_IRQn;
#endif
this would be more future proof I think
There was a problem hiding this comment.
I2C1_ER_IRQn is a enum value this could not be done.
cores/arduino/stm32/twi.c
Outdated
| #elif defined(STM32F0xx) || defined(STM32F1xx) || defined(STM32L1xx) | ||
| #define I2C_NUM (2) | ||
| #else // STM32F2xx || STM32F3xx || STM32F4xx || STM32L0xx | ||
| #define I2C_NUM (3) |
There was a problem hiding this comment.
What do you thin about
#elif defined( STM32F2xx) || defined(STM32F3xx) || defined(STM32F4xx) || #defined(STM32L0xx)
#else
#error Unknown Family - unknown I2C_NUM
#endif
so that 3 is not the default for any new family and we will be remonded to complete this part
| i2c_t *obj = get_i2c_obj(hi2c); | ||
|
|
||
| if(obj->isMaster == 0) { | ||
| HAL_I2C_EnableListen_IT(hi2c); |
Signed-off-by: fpr <[email protected]>
Issue #123 fixed.